From c810424c98c208a2427cfc4a4a435b31ad625d76 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 7 Oct 2016 12:34:00 -0700 Subject: [PATCH] Tweak message in Dependency::parse Provide some contextual information about why a dependency failed to parse. --- src/cargo/core/dependency.rs | 51 ++++++++++++++++++++++++----- src/cargo/core/registry.rs | 6 ++-- src/cargo/ops/cargo_install.rs | 9 +++-- src/cargo/sources/registry/index.rs | 4 +-- src/cargo/util/toml.rs | 11 ++++++- tests/resolve.rs | 11 +++---- 6 files changed, 65 insertions(+), 27 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index bd909b309..b96fed73f 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -88,12 +88,21 @@ impl Encodable for Kind { impl DependencyInner { /// Attempt to create a `Dependency` from an entry in the manifest. + /// + /// The `deprecated_extra` information is set to `Some` when this method + /// should *also* parse historically deprecated semver requirements. This + /// information allows providing diagnostic information about what's going + /// on. + /// + /// If set to `None`, then historically deprecated semver requirements will + /// all be rejected. pub fn parse(name: &str, version: Option<&str>, source_id: &SourceId, - config: &Config) -> CargoResult { + deprecated_extra: Option<(&PackageId, &Config)>) + -> CargoResult { let (specified_req, version_req) = match version { - Some(v) => (true, try!(DependencyInner::parse_with_deprecated(v, config))), + Some(v) => (true, try!(DependencyInner::parse_with_deprecated(v, deprecated_extra))), None => (false, VersionReq::any()) }; @@ -105,16 +114,31 @@ impl DependencyInner { }) } - fn parse_with_deprecated(req: &str, config: &Config) -> CargoResult { + fn parse_with_deprecated(req: &str, + extra: Option<(&PackageId, &Config)>) + -> CargoResult { match VersionReq::parse(req) { Err(e) => { + let (inside, config) = match extra { + Some(pair) => pair, + None => return Err(e.into()), + }; match e { ReqParseError::DeprecatedVersionRequirement(requirement) => { - let msg = format!("One of your version requirements ({}) is invalid. \ -Previous versions of Cargo accepted this malformed requirement, but it is being deprecated. Please \ -use {} instead.", req, requirement); + let msg = format!("\ +parsed version requirement `{}` is no longer valid + +Previous versions of Cargo accepted this malformed requirement, +but it is being deprecated. This was found when parsing the manifest +of {} {}, and the correct version requirement is `{}`. + +This will soon become a hard error, so it's either recommended to +update to a fixed version or contact the upstream maintainer about +this warning. +", + req, inside.name(), inside.version(), requirement); try!(config.shell().warn(&msg)); - + Ok(requirement) } e => Err(From::from(e)), @@ -238,8 +262,19 @@ impl Dependency { pub fn parse(name: &str, version: Option<&str>, source_id: &SourceId, + inside: &PackageId, config: &Config) -> CargoResult { - DependencyInner::parse(name, version, source_id, config).map(|di| { + let arg = Some((inside, config)); + DependencyInner::parse(name, version, source_id, arg).map(|di| { + di.into_dependency() + }) + } + + /// Attempt to create a `Dependency` from an entry in the manifest. + pub fn parse_no_deprecated(name: &str, + version: Option<&str>, + source_id: &SourceId) -> CargoResult { + DependencyInner::parse(name, version, source_id, None).map(|di| { di.into_dependency() }) } diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index bea9e6a10..854340077 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -364,7 +364,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { #[cfg(test)] pub mod test { use core::{Summary, Registry, Dependency}; - use util::{CargoResult, Config}; + use util::CargoResult; pub struct RegistryBuilder { summaries: Vec, @@ -405,13 +405,13 @@ pub mod test { } impl Registry for RegistryBuilder { - fn query(&mut self, dep: &Dependency, config: &Config) -> CargoResult> { + fn query(&mut self, dep: &Dependency) -> CargoResult> { debug!("querying; dep={:?}", dep); let overrides = self.query_overrides(dep); if overrides.is_empty() { - self.summaries.query(dep, config) + self.summaries.query(dep) } else { Ok(overrides) } diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 59d16e272..93ceb4040 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -57,7 +57,7 @@ pub fn install(root: Option<&str>, let map = try!(SourceConfigMap::new(config)); let (pkg, source) = if source_id.is_git() { try!(select_pkg(GitSource::new(source_id, config), source_id, - krate, vers, config, &mut |git| git.read_packages())) + krate, vers, &mut |git| git.read_packages())) } else if source_id.is_path() { let path = source_id.url().to_file_path().ok() .expect("path sources must have a valid path"); @@ -68,11 +68,11 @@ pub fn install(root: Option<&str>, specify an alternate source", path.display())) })); try!(select_pkg(PathSource::new(&path, source_id, config), - source_id, krate, vers, config, + source_id, krate, vers, &mut |path| path.read_packages())) } else { try!(select_pkg(try!(map.load(source_id)), - source_id, krate, vers, config, + source_id, krate, vers, &mut |_| Err(human("must specify a crate to install from \ crates.io, or use --path or --git to \ specify alternate source")))) @@ -251,7 +251,6 @@ fn select_pkg<'a, T>(mut source: T, source_id: &SourceId, name: Option<&str>, vers: Option<&str>, - config: &Config, list_all: &mut FnMut(&mut T) -> CargoResult>) -> CargoResult<(Package, Box)> where T: Source + 'a @@ -259,7 +258,7 @@ fn select_pkg<'a, T>(mut source: T, try!(source.update()); match name { Some(name) => { - let dep = try!(Dependency::parse(name, vers, source_id, config)); + let dep = try!(Dependency::parse_no_deprecated(name, vers, source_id)); let deps = try!(source.query(&dep)); match deps.iter().map(|p| p.package_id()).max() { Some(pkgid) => { diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index e7bd2b872..cda824676 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -135,9 +135,7 @@ impl<'cfg> RegistryIndex<'cfg> { name, req, features, optional, default_features, target, kind } = dep; - let dep = try!(DependencyInner::parse(&name, Some(&req), - &self.source_id, - self.config)); + let dep = try!(DependencyInner::parse(&name, Some(&req), &self.source_id, None)); let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") { "dev" => Kind::Development, "build" => Kind::Build, diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 06a169bed..b3de14096 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -336,6 +336,7 @@ impl TomlProject { } struct Context<'a, 'b> { + pkgid: Option<&'a PackageId>, deps: &'a mut Vec, source_id: &'a SourceId, nested_paths: &'a mut Vec, @@ -566,6 +567,7 @@ impl TomlManifest { { let mut cx = Context { + pkgid: Some(&pkgid), deps: &mut deps, source_id: source_id, nested_paths: &mut nested_paths, @@ -714,6 +716,7 @@ impl TomlManifest { let mut warnings = Vec::new(); let mut deps = Vec::new(); let replace = try!(self.replace(&mut Context { + pkgid: None, deps: &mut deps, source_id: source_id, nested_paths: &mut nested_paths, @@ -883,7 +886,13 @@ impl TomlDependency { }; let version = details.version.as_ref().map(|v| &v[..]); - let mut dep = try!(DependencyInner::parse(name, version, &new_source_id, cx.config)); + let mut dep = match cx.pkgid { + Some(id) => { + try!(DependencyInner::parse(name, version, &new_source_id, + Some((id, cx.config)))) + } + None => try!(DependencyInner::parse(name, version, &new_source_id, None)), + }; dep = dep.set_features(details.features.unwrap_or(Vec::new())) .set_default_features(details.default_features.unwrap_or(true)) .set_optional(details.optional.unwrap_or(false)) diff --git a/tests/resolve.rs b/tests/resolve.rs index 56ae25c62..9129a1e6e 100644 --- a/tests/resolve.rs +++ b/tests/resolve.rs @@ -10,7 +10,7 @@ use hamcrest::{assert_that, equal_to, contains}; use cargo::core::source::{SourceId, GitReference}; use cargo::core::dependency::Kind::{self, Development}; use cargo::core::{Dependency, PackageId, Summary, Registry}; -use cargo::util::{CargoResult, ToUrl, Config}; +use cargo::util::{CargoResult, ToUrl}; use cargo::core::resolver::{self, Method}; fn resolve(pkg: PackageId, deps: Vec, @@ -33,8 +33,7 @@ impl ToDep for &'static str { fn to_dep(self) -> Dependency { let url = "http://example.com".to_url().unwrap(); let source_id = SourceId::for_registry(&url); - let config = Config::default().unwrap(); - Dependency::parse(self, Some("1.0.0"), &source_id, &config).unwrap() + Dependency::parse_no_deprecated(self, Some("1.0.0"), &source_id).unwrap() } } @@ -102,16 +101,14 @@ fn dep(name: &str) -> Dependency { dep_req(name, "1.0.0") } fn dep_req(name: &str, req: &str) -> Dependency { let url = "http://example.com".to_url().unwrap(); let source_id = SourceId::for_registry(&url); - let config = Config::default().unwrap(); - Dependency::parse(name, Some(req), &source_id, &config).unwrap() + Dependency::parse_no_deprecated(name, Some(req), &source_id).unwrap() } fn dep_loc(name: &str, location: &str) -> Dependency { let url = location.to_url().unwrap(); let master = GitReference::Branch("master".to_string()); let source_id = SourceId::for_git(&url, master); - let config = Config::default().unwrap(); - Dependency::parse(name, Some("1.0.0"), &source_id, &config).unwrap() + Dependency::parse_no_deprecated(name, Some("1.0.0"), &source_id).unwrap() } fn dep_kind(name: &str, kind: Kind) -> Dependency { dep(name).clone_inner().set_kind(kind).into_dependency() -- 2.30.2